Skip to content

Move existing PPR sampler work off event loop - #708

Merged
mkolodner-sc merged 2 commits into
mainfrom
mkolodner-sc/pr701-ppr-event-loop
Jul 27, 2026
Merged

Move existing PPR sampler work off event loop#708
mkolodner-sc merged 2 commits into
mainfrom
mkolodner-sc/pr701-ppr-event-loop

Conversation

@mkolodner-sc

@mkolodner-sc mkolodner-sc commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Moves the existing distributed PPR sampler's CPU-heavy C++ work off the asyncio event-loop path.

This PR is limited to the existing/non-typed PPR path. Typed-channel PPR enablement is left for the next PR in the stack.

Stack

Reasoning for Change

PPR sampling runs several CPU-heavy C++ steps while async graph-store/RPC work is also in flight. When those steps run on the event-loop thread or hold the GIL longer than necessary, they can delay RPC completion callbacks and reduce concurrency across sampler coroutines.

This change keeps the PPR math and returned sequences unchanged while moving construction, queue draining, residual pushes, and extraction through executor/GIL-release boundaries where possible.

Changes

  • Releases the GIL around PPRForwardPush construction, queue draining, residual push, and extraction in the pybind layer.
  • Runs PPR state construction, drain_queue, push_residuals, and top-k extraction through run_in_executor from the Python sampler.
  • Removes a redundant frontier .to(device) before one-hop fetches; extracted tensors are still moved to the sampler device at the existing output boundary.

@mkolodner-sc
mkolodner-sc force-pushed the mkolodner-sc/pr701-ppr-cleanup branch from 5197b5d to 7f6d9c0 Compare July 15, 2026 20:02
@mkolodner-sc
mkolodner-sc force-pushed the mkolodner-sc/pr701-ppr-event-loop branch from fc410ab to 2b85456 Compare July 15, 2026 20:02
@mkolodner-sc
mkolodner-sc force-pushed the mkolodner-sc/pr701-ppr-cleanup branch from 7f6d9c0 to 19c0d92 Compare July 15, 2026 21:24
@mkolodner-sc
mkolodner-sc force-pushed the mkolodner-sc/pr701-ppr-event-loop branch from 2b85456 to ecb394c Compare July 15, 2026 21:24
@mkolodner-sc
mkolodner-sc force-pushed the mkolodner-sc/pr701-ppr-cleanup branch from 19c0d92 to f7aec06 Compare July 15, 2026 21:40
@mkolodner-sc
mkolodner-sc force-pushed the mkolodner-sc/pr701-ppr-event-loop branch from ecb394c to 0047b2f Compare July 15, 2026 21:40

@kmontemayor2-sc kmontemayor2-sc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Matt!

Comment thread gigl-core/core/sampling/python_ppr_forward_push.cpp Outdated
Comment thread gigl-core/core/sampling/python_ppr_forward_push.cpp
Comment thread gigl/distributed/dist_ppr_sampler.py
Comment thread gigl/distributed/dist_ppr_sampler.py
Comment thread gigl/distributed/dist_ppr_sampler.py
@mkolodner-sc
mkolodner-sc force-pushed the mkolodner-sc/pr701-ppr-event-loop branch 3 times, most recently from 60a0cea to ff2d3ea Compare July 15, 2026 23:04

@kmontemayor2-sc kmontemayor2-sc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Nice work Matt :)

Do you think there's similar low-hanging (but hard to find!) fruit here for the khop sampler?

E.g. is it possible to put this bit in the background? https://github.com/alibaba/graphlearn-for-pytorch/blob/main/graphlearn_torch/python/distributed/dist_neighbor_sampler.py#L297

Or do we need to also write a CPP version of that _sample_from_nodes so we can force-detach the gil? May not be difficult? WDYT?

Comment thread gigl/distributed/dist_ppr_sampler.py
Comment thread gigl/distributed/dist_ppr_sampler.py
@mkolodner-sc
mkolodner-sc force-pushed the mkolodner-sc/pr701-ppr-event-loop branch from ff2d3ea to d0f9cea Compare July 16, 2026 18:46
@mkolodner-sc
mkolodner-sc force-pushed the mkolodner-sc/pr701-ppr-cleanup branch 2 times, most recently from 2dd7085 to 68d6d62 Compare July 16, 2026 19:15
@mkolodner-sc
mkolodner-sc force-pushed the mkolodner-sc/pr701-ppr-event-loop branch 2 times, most recently from 33c8dde to 1b0ea2a Compare July 16, 2026 19:21
@mkolodner-sc
mkolodner-sc force-pushed the mkolodner-sc/pr701-ppr-cleanup branch 2 times, most recently from 3d34854 to f5c430d Compare July 16, 2026 20:01
@mkolodner-sc
mkolodner-sc force-pushed the mkolodner-sc/pr701-ppr-event-loop branch from 1b0ea2a to ed7387c Compare July 16, 2026 20:01
Base automatically changed from mkolodner-sc/pr701-ppr-cleanup to main July 17, 2026 09:13
@mkolodner-sc
mkolodner-sc force-pushed the mkolodner-sc/pr701-ppr-event-loop branch from ed7387c to 457c30b Compare July 27, 2026 10:20
@mkolodner-sc
mkolodner-sc added this pull request to the merge queue Jul 27, 2026
Merged via the queue into main with commit c3f2ae5 Jul 27, 2026
7 checks passed
@mkolodner-sc
mkolodner-sc deleted the mkolodner-sc/pr701-ppr-event-loop branch July 27, 2026 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants